Skip to content

Flexbox: fix container baseline for column-reverse containers - #1127

Merged
nicoburns merged 1 commit into
mainfrom
devin/1787096013-flex-baseline-wrap-reverse
Aug 18, 2026
Merged

Flexbox: fix container baseline for column-reverse containers#1127
nicoburns merged 1 commit into
mainfrom
devin/1787096013-flex-baseline-wrap-reverse

Conversation

@nicoburns

Copy link
Copy Markdown
Member

Objective

Fix the flex container baseline computation so that it matches browsers (and the current css-flexbox-1 ED wording, https://drafts.csswg.org/css-flexbox-1/#flex-baselines) for reverse-direction containers. This is what causes Blitz to fail WPT tests css/css-flexbox/flexbox-baseline-multi-line-horiz-004.html and css/css-flexbox/flexbox-baseline-multi-line-vert-002.html.

Context

The original hypothesis was that the wrap-reverse line selection was wrong (that the first baseline should come from the first flex line in flex-line order regardless of wrap direction, per the css-flexbox-1 TR wording). However, empirical testing against Chrome (both via gentest fixtures with the container nested in a baseline-aligning flex parent, and via headless Chrome measurements of the inline baseline export) shows Chrome consistently generates the first baseline from the visually startmost line — for wrap-reverse that is the last line in flex-line order, which is what taffy already does. The current ED spec text agrees: "startmost flex line", where startmost links to the writing-mode (visual) definition of start.

The actual bug making the two WPT tests fail is on the column side: for column-reverse containers taffy took the baseline of the first item in flex order, which is positioned at the visual bottom. Browsers use the startmost (visually topmost) item, which for reverse-direction containers is the last item in flex order. In the WPT reftests it is the reference page (which uses column-reverse / single-line column containers) that taffy renders wrong, producing the observed ~15px baseline offset.

Pseudo-diff of the fix in compute_flexbox_layout:

  // line selection unchanged (visually startmost line):
  let first_line = if constants.is_wrap_reverse { flex_lines.last() } else { flex_lines.first() };
  let first_vertical_baseline = first_line.and_then(|line| {
-     line.items.iter()
-         .find(|item| constants.is_column || item.participates_in_baseline_alignment(constants.dir))
-         .or_else(|| line.items.iter().next())
-         .map(|child| child.baseline)
+     if constants.is_column {
+         // startmost item: last in flex order for reverse-direction containers
+         let item = if constants.dir.is_reverse() { line.items.last() } else { line.items.first() };
+         item.map(|child| child.baseline)
+     } else {
+         line.items.iter()
+             .find(|item| item.participates_in_baseline_alignment(constants.dir))
+             .or_else(|| line.items.iter().next())
+             .map(|child| child.baseline)
+     }
  });

New generated test fixtures cover:

  • row wrap-reverse multi-line first baseline (with and without a text item participating in baseline alignment) — regression tests locking in the existing (correct) behavior
  • column wrap-reverse multi-line
  • column-reverse single-line (previously wrong)
  • column-reverse + wrap-reverse multi-line (previously wrong)

All fixtures regenerated from Chrome via just gentest; cargo test --workspace, cargo fmt and cargo clippy pass.

Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/1674564446a64939b4972322b4f46da4
Requested by: @nicoburns

@staging-devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@staging-devin-ai-integration

Copy link
Copy Markdown

Blitz WPT comparison (full css/css-flexbox suite, taffy main vs this PR via local patch):

  • Test-level: unchanged — 1238 run, 851 passed, 387 failed on both.
  • Subtest-level: +3 passes with this PR (2531 → 2534), all in css/css-flexbox/alignment/flex-align-baseline-flex-001.html (subtests .target > * 13/29/45, FAIL → PASS). No regressions.
  • The originally-cited flexbox-baseline-multi-line-horiz-004.html and flexbox-baseline-multi-line-vert-002.html already PASS on Blitz with taffy main and remain passing with this PR.

@nicoburns
nicoburns enabled auto-merge (squash) August 18, 2026 23:54
@nicoburns
nicoburns force-pushed the devin/1787096013-flex-baseline-wrap-reverse branch from f4212c1 to 26cc1ae Compare August 18, 2026 23:54
@nicoburns
nicoburns merged commit 8b01c6a into main Aug 18, 2026
28 checks passed
@nicoburns
nicoburns deleted the devin/1787096013-flex-baseline-wrap-reverse branch August 18, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant